Skip to content

MDEV-37316#5073

Draft
ParadoxV5 wants to merge 6 commits into
mainfrom
MDEV-37316
Draft

MDEV-37316#5073
ParadoxV5 wants to merge 6 commits into
mainfrom
MDEV-37316

Conversation

@ParadoxV5
Copy link
Copy Markdown
Contributor

Just a draft/preview that also serves as CI upload and backup; not final yet, not even close…


What problem is the patch trying to solve?
If some output changed that is not visible in a test case, what was it looking like before the change and how it's looking with this patch applied?
Do you think this patch might introduce side-effects in other parts of the server?
Anything from release notes?

To prepare for MDEV-37321, this commit moves them to a
`Remote_event_stream` class outside of `sql/`.
The `.cc` file will switch to Connector/C unbeknownst to `rpl_mi.cc`.

Only the following steps are reörganized to reusable instance methods:
* Connection setup and startup
* VIO management
  * Abort when the thread is killed
  * Closing
    (already covered by Connector/C, but not by the current `libmysql`)

The following steps are currently not öptimal for moving
(have verbose changesets), so they are only wrapped, not reörganized:
* steps that run queries *with copy-pasted code*
* sending commands `COM_REGISTER_SLAVE` & `COM_BINLOG_DUMP`,
  including serializing their arguments
  * Connector/C does not have a formal API for serializing
    command arguments, especially variable-length strings.
  * Connector/C has a Binlog API that duplicates this step, but that
    API is not modular for the Server’s full replication capabilities.
* error checking

These require refactors and ideally an evaluation for `mariadb-binlog`.

To enable this migration, this commit additionally:
* Reserves the management of the IO thread’s VIO to the client library
* Draft-specific: omits MDEV-28114’s kludge failure
  recovery for Semi-Sync graceful self-KILL since
  * MDEV-32551 fixed the root bug on the ACK receiver, and
  * MDEV-39583 plans to remove this entire sub-component.

Draft-specific: Until Connector/C exports the pre-decompress (network)
byte count, `@@read_binlog_speed_limit` will measure the post-decompress
count even in `@@slave_compressed_protocol` for now.
@ParadoxV5 ParadoxV5 added MariaDB Corporation Replication Patches involved in replication labels May 13, 2026
gemini-code-assist[bot]

This comment was marked as spam.

ParadoxV5 added 5 commits May 14, 2026 19:43
also, better say preparing for 37316, not 37321
Draft-specific: Zero-config TLS is (temporarily?) unavailable.
Compile `rpl/` separately from the rest of the server (`sql/`),
so it can specify linking to Connector/C instead of `sql-common/`.

`mariadb_capi_rename.h` had to add a lot of symbols to resolve
multiple-definition errors with the addition of Connector/C.
While linking to the dynamic library instead of a static
compile could hide the ODR violations, running the server
can still select the wrong version and result in surprises.
This phenomenon affects not only this commit,
but also plugins that link to Connector/C themselves.
Only GCC’s C++ standard library[^l] accepts `uchar`s
(`std::basic_string`(`_view`)`<unsigned char>`). Awesome!

[^l]: an intentional choice to avoid the
server library’s messy web of dependencies
`uchar`’s cousin, `ulong`!

In `Remote_event_stream::send_command()`,
`strlen` also needs casting to `ulong` before the switch to Connector/C.
@ParadoxV5
Copy link
Copy Markdown
Contributor Author

Okay, does CI for some genius reason not use CMakeLists.txt that my local build recognizes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

MariaDB Corporation Replication Patches involved in replication

Development

Successfully merging this pull request may close these issues.

1 participant